Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade project to .NET Standard 2.0 #5

Merged

Conversation

phillip-haydon
Copy link
Contributor

Upgraded the projects:

  • NVelocity > .NET Standard 2.0
  • NVelocity.Tests > .NET Core 2.0

Also:

  • Upgrade to NUnit 3.10.1
  • Added Ubuntu as an image on AppVeyor
  • Added Artifact for NuGet package

https://ci.appveyor.com/project/phillip-haydon/nvelocity/build/1.0.28


Console.WriteLine(writer);

Assert.Throws<ParseErrorException>(() => velocityEngine.GetTemplate(GetFileName(null, "nv14", TemplateTest.TMPL_FILE_EXT)));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When converting to NUnit 3, I had to rewrite the test because of the removal of ExpectedException

The test actually fails with the expected exception when calling GetTemplate, so everything under that was never run. I just removed it since it didn't seem necessary.

@@ -68,7 +68,7 @@ public struct TemplateTest

static TemplateTest()
{
FILE_RESOURCE_LOADER_PATH = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, Path.Combine(ConfigurationManager.AppSettings["tests.src"], "templates"));
FILE_RESOURCE_LOADER_PATH = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "templates");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This configuration was empty in the AppSettings so I just removed it instead of attempting to create new configurations.

@@ -17,7 +17,6 @@ namespace NVelocity
using System;
using System.Collections;
using System.Data;
using System.Data.OleDb;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was referenced and not used, unsure if .NET Core has an equiv but removing it didn't break the tests.

project: NVelocity.sln
publish_nuget: false
build_script:
- dotnet build ./NVelocity.sln -c Release
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read through the Core project and it seemed a little bit overkill (for now) to pull everything into a separate build file, and I was having trouble running it on Ubuntu. So I just put it in here for now.

@phillip-haydon
Copy link
Contributor Author

Nudge @jonorossi for feedback :)

@jonorossi
Copy link
Member

Looks great. Let's get this merged for now, but the build output is producing the file build/NVelocity.1.0.0.nupkg, that version is coming from somewhere as I would have expected 0.0.x like the CI builds for the other projects.

@jonorossi jonorossi merged commit eca20dc into castleproject:master Aug 6, 2018
@phillip-haydon phillip-haydon deleted the upgrade-to-netstandard branch August 8, 2018 05:56
@jonorossi jonorossi added this to the v1.2.0 milestone Dec 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants